EFM Recovery Integration Tests: Part 1 #6156

kc1116 · 2024-06-26T15:53:14Z

This PR adds an integration test that partially tests the EFM recovery governance transaction. The intention behind this PR is to introduce a functional test suite that covers the entire process, including generating and submitting the governance transaction, making changes to the Epoch smart contract, and observing the emission of an EFM recovery service event. However, it does not test whether the emitted EFM service event is processed by the fallback state machine due to pending updates required for downstream components to deliver the service event to the fallback state machine. This approach allows us to detect any transaction argument generation or submission issues, as well as container orchestration challenges encountered while manually triggering EFM, while we continue to finalize the remaining EFM tasks.

Start a flow test network and manually force network into EFM
Generate transaction arguments for recover epoch transaction
Submit recover epoch transaction
Observe epoch recover service event

- access-address: address of access node used to retrieve snapshot - access-network-key: network key of the access node - insecure: use insecure API endpoint - out: output dir for json file

Co-authored-by: Jordan Schalm <[email protected]>

- add epoch address to env template - update recover transaction template

codecov-commenter · 2024-06-26T15:56:31Z

Codecov Report

Attention: Patch coverage is 23.93162% with 178 lines in your changes missing coverage. Please review.

Project coverage is 41.67%. Comparing base (dbaa1ce) to head (1d2e034).

Files	Patch %	Lines
cmd/bootstrap/run/epochs.go	0.00%	100 Missing ⚠️
cmd/util/cmd/epochs/cmd/recover.go	61.90%	11 Missing and 5 partials ⚠️
integration/utils/arguments.go	0.00%	16 Missing ⚠️
integration/utils/transactions.go	0.00%	15 Missing ⚠️
utils/unittest/service_events_fixtures.go	0.00%	13 Missing ⚠️
cmd/bootstrap/utils/key_generation.go	0.00%	11 Missing ⚠️
model/convert/service_event.go	40.00%	2 Missing and 1 partial ⚠️
integration/testnet/network.go	50.00%	1 Missing and 1 partial ⚠️
utils/unittest/execution_state.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                    @@
##           feature/efm-recovery    #6156      +/-   ##
========================================================
- Coverage                 41.71%   41.67%   -0.04%     
========================================================
  Files                      1976     1978       +2     
  Lines                    139331   139430      +99     
========================================================
- Hits                      58120    58111       -9     
- Misses                    75155    75273     +118     
+ Partials                   6056     6046      -10

Flag	Coverage Δ
unittests	`41.67% <23.93%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- remove script - move cluster qc contract address to argument - remove debug line

jordanschalm

Didn't quite get through the whole PR yet. Posting what I have reviewed so far as I'll be out on Monday.

integration/tests/epochs/base_suite.go

cmd/util/cmd/epochs/cmd/recover.go

integration/tests/epochs/recover_epoch/recover_epoch_efm_test.go

jordanschalm · 2024-06-29T01:01:42Z

integration/tests/epochs/recover_epoch/recover_epoch_efm_test.go

+	// pausing consensus node will force the network into EFM
+	ln := s.GetContainersByRole(flow.RoleCollection)[0]
+	_ = ln.Pause()


The comment says consensus node, but we're pausing the collection node.

In addition, I am guessing that EFM is being triggered only because we have made the DKG phases much shorter (10 views).

We should test this without pausing the collection node:

if we still trigger EFM, then we don't need to pause the collection node and can remove that code

if we don't trigger EFM, then we should revert the change to the DKG phase length

The test did not work without pausing the collection node, we can look into it further in part 2.

integration/tests/epochs/recover_epoch/recover_epoch_efm_test.go

jordanschalm · 2024-06-29T01:05:43Z

integration/tests/epochs/recover_epoch/recover_epoch_efm_test.go


-	sns := s.GetContainersByRole(flow.RoleConsensus)
-	_ = sns[0].Pause()
+	// get the latest snapshot and start new container with it


outdated comment - we aren't starting a new container

durkmurder · 2024-07-03T16:39:47Z

integration/tests/epochs/base_suite.go

@@ -117,8 +118,15 @@ func (s *BaseSuite) SetupTest() {
 	go lib.LogStatusPeriodically(s.T(), s.Ctx, s.log, s.Client, 5*time.Second)
 }

+func (s *BaseSuite) TearDownTest() {
+	s.log.Info().Msg("================> Start TearDownTest")


still need those logs?

Yes these help with debugging we have them on all the epoch related integration tests.

model/convert/service_event.go

…flow/flow-go into khalil/efm-recovery-integration-part1

… EFM without pausing LN container

…ion-part1

- update state comittment const

… khalil/efm-recovery-integration-part1

…ion-part1

kc1116 and others added 15 commits June 26, 2024 11:36

trigger epoch fallback mode manually in test

d9a99c9

add flags

ebe557a

- access-address: address of access node used to retrieve snapshot - access-network-key: network key of the access node - insecure: use insecure API endpoint - out: output dir for json file

add execute cmd func

0d5dcf5

write node infos pub to dir

a842760

add flag output dir

c3a05be

generate transaction args and submit recover epoch transaction

0d0378c

Apply suggestions from code review

f0670d0

Co-authored-by: Jordan Schalm <[email protected]>

move warning log

ee7fa73

update comments

09f29e7

Update recover_epoch_efm_test.go

96b9776

Update suite.go

d48d438

add ClusterQCVoteData conversion to cdc arg

d75354b

update test

b93e8ac

- add epoch address to env template - update recover transaction template

update cluster qc vote data address, fix epoch configuration

ec964a8

fix service event decoding

d0c40f2

kc1116 added 4 commits June 27, 2024 17:02

write node node pub infos file when test network is bootstrapped

d3d7af6

update test godocs

4be8e65

update test components

62028cd

- remove script - move cluster qc contract address to argument - remove debug line

Update clusters.go

7a3b6b9

kc1116 marked this pull request as ready for review June 28, 2024 00:27

kc1116 requested a review from zhangchiqing as a code owner June 28, 2024 00:27

kc1116 requested review from durkmurder, jordanschalm and AlexHentschel and removed request for zhangchiqing June 28, 2024 00:27

update go.mod(s)

4ba9d6a

jordanschalm reviewed Jun 29, 2024

View reviewed changes

durkmurder reviewed Jul 3, 2024

View reviewed changes

model/convert/service_event.go Outdated Show resolved Hide resolved

kc1116 added 15 commits July 15, 2024 12:41

add godoc to ConvertClusterQcsCdc

d2fb6bc

rename newFlowClusterQCVoteDataStructType -> newClusterQCVoteDataCdcType

0d225b7

Update epochs.go

216ca7e

Merge branch 'khalil/efm-recovery-integration-part1' of github.com:on…

a650821

…flow/flow-go into khalil/efm-recovery-integration-part1

remove duplicate code update test

e9532ef

get current epoch target time from snapshot

a0df4a2

remove outdated comments "pausing container"

c331ab2

add EFM TODO to investigate short DKG phase len config not triggering…

01bfe0d

… EFM without pausing LN container

added root go.sum

4a2a888

added insecure and integration go.sum

a982d13

fix lint

3aaf62f

Update key_generation.go

3b5c953

Merge branch 'feature/efm-recovery' into khalil/efm-recovery-integrat…

a614c62

…ion-part1

fix suite.go imports

f1db034

fix recover event fixture

00e2f40

- update state comittment const

kc1116 requested a review from ramtinms as a code owner July 17, 2024 21:23

Merge branch 'feature/efm-recovery' of github.com:onflow/flow-go into…

df64765

… khalil/efm-recovery-integration-part1

kc1116 removed the request for review from ramtinms July 17, 2024 21:48

kc1116 added 10 commits July 17, 2024 17:59

Update suite.go

5a4cd70

Update suite.go

1eb81e8

Update suite.go

c60abe3

Update suite.go

b51a73f

Update suite.go

2ae7bd7

Update suite.go

5b92090

Update suite.go

34a49bf

Merge branch 'feature/efm-recovery' into khalil/efm-recovery-integrat…

772133f

…ion-part1

Merge branch 'feature/efm-recovery' into khalil/efm-recovery-integrat…

d1fab36

…ion-part1

Update base_suite.go

1d2e034

kc1116 merged commit 0a30e45 into feature/efm-recovery Jul 19, 2024
55 checks passed

kc1116 deleted the khalil/efm-recovery-integration-part1 branch July 19, 2024 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EFM Recovery Integration Tests: Part 1 #6156

EFM Recovery Integration Tests: Part 1 #6156

kc1116 commented Jun 26, 2024 •

edited

Loading

codecov-commenter commented Jun 26, 2024 •

edited

Loading

jordanschalm left a comment

jordanschalm Jun 29, 2024

kc1116 Jul 10, 2024

jordanschalm Jun 29, 2024

kc1116 Jul 10, 2024

durkmurder Jul 3, 2024

kc1116 Jul 10, 2024

EFM Recovery Integration Tests: Part 1 #6156

EFM Recovery Integration Tests: Part 1 #6156

Conversation

kc1116 commented Jun 26, 2024 • edited Loading

codecov-commenter commented Jun 26, 2024 • edited Loading

Codecov Report

jordanschalm left a comment

Choose a reason for hiding this comment

jordanschalm Jun 29, 2024

Choose a reason for hiding this comment

kc1116 Jul 10, 2024

Choose a reason for hiding this comment

jordanschalm Jun 29, 2024

Choose a reason for hiding this comment

kc1116 Jul 10, 2024

Choose a reason for hiding this comment

durkmurder Jul 3, 2024

Choose a reason for hiding this comment

kc1116 Jul 10, 2024

Choose a reason for hiding this comment

kc1116 commented Jun 26, 2024 •

edited

Loading

codecov-commenter commented Jun 26, 2024 •

edited

Loading